Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow sample weight in shuffle features #662

Merged
merged 2 commits into from
Apr 27, 2023
Merged

allow sample weight in shuffle features #662

merged 2 commits into from
Apr 27, 2023

Conversation

solegalli
Copy link
Collaborator

closes #654

@solegalli
Copy link
Collaborator Author

Hey @markdregan

Here a PR to expand the fit method of Shuffle features to accommodate sample weights.

This is training the model with sample weights. The predictions done after shuffling are done without weighting. This is, based on my understanding, the correct procedure. Correct me if I am wrong.

Thank you!

@solegalli
Copy link
Collaborator Author

Looking at the cross_validate logic, the performance on test set is calculated without sample_weights:

https://github.com/scikit-learn/scikit-learn/blob/364c77e047ca08a95862becf40a04fe9d4cd2c98/sklearn/model_selection/_validation.py#L707-L711

Sample weights is used only to train:

https://github.com/scikit-learn/scikit-learn/blob/364c77e047ca08a95862becf40a04fe9d4cd2c98/sklearn/model_selection/_validation.py#L682-L686

@markdregan
Copy link

Thank you! I will test this week. 🙌

@markdregan
Copy link

TY. Confirmed working correctly. Thanks for the fast PR!

@solegalli solegalli merged commit 4d55ed7 into main Apr 27, 2023
@solegalli solegalli deleted the allow_fit_params branch April 27, 2023 07:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fit_params not supported for SelectByShuffling (documentation suggests it is supported)
2 participants